Upgrading fermipy to numpy>2 and astropy>7#679
Conversation
Fill missing (masked) values in key columns before joining In astropy >v6 table.join() strictly rejects missing values in key columns while it was cool with this in astropy v5.
|
if the tests are running with numpy 1.26, they might fail. One would need to upgrade the CI environment to astropy >7, numpy 2>, gammapy>2 |
python 3.9 could be removed from CI |
|
Thanks @facero! I just dropped support for which is called by |
numpy2 has stricter handling of scalar conversion, where 1-element arrays are no longer implicitly converted to Python scalars. In [7]: pix[0:1].shape Out[7]: (1,) int(pix[0:1]) # will throw an error int(pix[0:1][0] # works
|
ok welln you fix one thing here and another one breaks there ;-) I opened a discussion to see if it's gammapy related : Which gammapy versin do you have no in the CI ? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #679 +/- ##
==========================================
- Coverage 51.23% 51.12% -0.11%
==========================================
Files 145 145
Lines 27436 27443 +7
==========================================
- Hits 14056 14031 -25
- Misses 13380 13412 +32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Besides this I've added new np2 fixes while testing more fermipy functions (gta.optimize, localize,extension, etc). I found an issue for for This is caused by NumPy 2.x's stricter handling of scalar conversion, where 1-element arrays are no longer implicitly converted to Python scalars. Fixing it by specifying : |
PyLike does not accept np.float32 as input, only float objects.
|
another issue in The issue is that PyLike expects a float as an input but numpy2 now returns a Fixing this by specifying |
|
Ok I manage to run a script calling most of fermipy functions and now it completes without issues with np2 and astropy7 hourray ✅ I have not tested all fermipy function with all possible options, so we don't have 100% coverage but I'll say it covers for now most use cases. This PR currently works with the following versions : I also attach here the small setup in case you want to test it. W44-test.zip |
Fix max version possible of regions, astropy and scipy Bump up numpy and gammapy min version
not only the env-dev
|
Thanks @facero ! We actually have a |
This PR proposes to fix remaining issues with numpy2 and also issues that I discovered with astropy 6&7.
This follows the PR fixing numpy2 in the fermitools and this PR should be tested in a numpy and astropy 7 environment in conjunction with the fermitools numpy2 version.
Issues solved:
All issues (for now) were related to
catalog.py.I haven't yet tested a longer fermipy analysis than just
gta.setup().I will try later and report if any other bugs are found.
So far this is what I've done.
Numpy 2 related
As numpy2 deprecated the
np.string_call here was failing.the fix was to use :
(np.bytes_, np.str_)to handle both string and bytes columnsMaybe just
np.str_was enough. But at least we should be safe with both.Astropy >6 related
In astropy >6 ,
table.join()here strictly rejects missing values in key columns. The join_tables() function in catalog.py was calling join() directly on tables where the left key column (Extended_Source_Name) had masked/NaN values for non-extended sources, causing:astropy.table.np_utils.TableMergeError: Left key column 'Extended_Source_Name' has missing valuesThe fix here is more painful and I'm not completly sure that the proposed solution is the best.
The solution is to add a pre-processing step before the
join()call that fills masked values in the key column for both tables with 0 or empty strings.Maybe this entire
join_tablesfunction could be revisited and simplified as it is a bit cryptic now.But let's try this fix first.